Skip to content

Conversation

@cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Aug 6, 2024

Summary

As we've established in #7486, color computations are expensive to recalculate repeatedly and should be memoized where possible. This is fine for component styles (#7529), but our various euiComponentVariables utilities are not getting memoized except as part of component styles.

euiFormVariables is particularly chonky and likely bears examining more closely, but in the interim I'm solving this particular microperf irritant by creating a separate utility just to grab the maxWidth size for components that only need this token (a surprisingly high number).

My upcoming EuiComboBox conversion will be utilizing this PR as well for cleaner and more performant code ✨

QA

General checklist

  • Browser QA - N/A, fairly straightforward PR that doesn't need excessive cross-browser testing
  • Docs site QA - N/A
  • Code quality checklist - N/A, styles only
  • Release checklist - N/A, skipping a changelog as this is mostly tech debt/ an internal concern only
  • Designer checklist - N/A

…ed the single variable

- perf: color computations are expensive, and we don't need to calculate every single form var repeatedly for just one thing that's reused often
@cee-chen cee-chen added skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) emotion performance labels Aug 6, 2024
@cee-chen cee-chen marked this pull request as ready for review August 6, 2024 19:22
@cee-chen cee-chen requested a review from a team as a code owner August 6, 2024 19:22
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 🐈‍⬛ LGTM! If it became a larger performance issue across the board, we could consider separating colors vars and/or memoizing variables in general 🤔

Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look great! 🚢

@cee-chen
Copy link
Contributor Author

cee-chen commented Aug 7, 2024

Thanks y'all!

@cee-chen cee-chen merged commit 005c64a into elastic:main Aug 7, 2024
@cee-chen cee-chen deleted the emotion/form-max-width branch August 7, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emotion performance skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants